-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chains Merkle shreds in fail-entry-verification broadcast #35060
chains Merkle shreds in fail-entry-verification broadcast #35060
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #35060 +/- ##
=========================================
- Coverage 81.6% 81.6% -0.1%
=========================================
Files 830 830
Lines 224952 225053 +101
=========================================
+ Hits 183698 183743 +45
- Misses 41254 41310 +56 |
96536a9
to
8bc59cd
Compare
debug_assert_eq!(slot, 0u64); | ||
return Ok(Hash::default()); | ||
} | ||
debug_assert!(parent < slot, "parent: {parent} >= slot: {slot}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason to only make these debug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that if the this does not fail in tests then we should be good.
So the extra panic
code in release builds is unnecessary.
@@ -96,6 +99,34 @@ pub(super) fn recv_slot_entries(receiver: &Receiver<WorkingBankEntry>) -> Result | |||
}) | |||
} | |||
|
|||
// Returns the Merkle root of the last erasure batch of the parent slot. | |||
pub(super) fn get_chained_merkle_root( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function might be more intuitive if it was named get_chained_merkle_root_from_parent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar feedback as #35061 . LGTM, but could probably have some more helpful error messages for failed unwraps
8bc59cd
to
6dc8ecb
Compare
Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis. |
The commit migrates turbine/src/broadcast_stage/fail_entry_verification_broadcast_run.rs to use chained Merkle shreds variant. (cherry picked from commit 8fb389f)
…kport of #35060) (#35270) chains Merkle shreds in fail-entry-verification broadcast (#35060) The commit migrates turbine/src/broadcast_stage/fail_entry_verification_broadcast_run.rs to use chained Merkle shreds variant. (cherry picked from commit 8fb389f) Co-authored-by: behzad nouri <[email protected]>
Problem
Chain Merkle shreds in
turbine/src/broadcast_stage/fail_entry_verification_broadcast_run.rs
.Summary of Changes
Chained Merkle shreds in fail-entry-verification broadcast.